Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default skills #282

Merged
merged 4 commits into from
Mar 7, 2023
Merged

Add default skills #282

merged 4 commits into from
Mar 7, 2023

Conversation

NeonDaniel
Copy link
Member

@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Merging #282 (805dcd7) into dev (6ceb058) will increase coverage by 1.83%.
The diff coverage is 43.17%.

@@            Coverage Diff             @@
##              dev     #282      +/-   ##
==========================================
+ Coverage   50.35%   52.19%   +1.83%     
==========================================
  Files         119      156      +37     
  Lines       10077     8281    -1796     
==========================================
- Hits         5074     4322     -752     
+ Misses       5003     3959    -1044     
Impacted Files Coverage Δ
mycroft/audio/__main__.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/__main__.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/arduino.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/eyes.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/mouth.py 0.00% <0.00%> (ø)
mycroft/client/speech/__main__.py 0.00% <0.00%> (ø)
mycroft/client/speech/hotword_factory.py 0.00% <0.00%> (-88.89%) ⬇️
mycroft/client/speech/service.py 0.00% <0.00%> (ø)
mycroft/client/speech/silence.py 0.00% <0.00%> (-42.86%) ⬇️
mycroft/client/text/__init__.py 0.00% <0.00%> (ø)
... and 154 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@NeonDaniel NeonDaniel mentioned this pull request Mar 6, 2023
93 tasks
@NeonDaniel NeonDaniel marked this pull request as ready for review March 6, 2023 20:21
Copy link

@ChanceNCounter ChanceNCounter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I should transfer the Dismissal skill to OVOS (should have done a while ago) and ship it with essential, but I don’t think that needs to block 0.0.7

neon-skill-alerts~=1.2
ovos-skill-personal
ovos-skill-naptime
ovos-skill-date-time>=0.2.2a1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this particular specification will cause pip to pull any published prerelease, not just those of the pinned version

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik any a in the dependencies will make it so pip considers alpha dependencies.. i.e. package~=0.1,>=0.0.2a1 would expand to pip install package >=0.1.0,<1.0.0,>=0.0.2a1 which means all alphas are added to candidate versions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. What I'm concerned about is that 0.0.3a1 will also be captured. If that's what we want, so be it. I've only recently started to get my head around pip and PEP 440

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's important to make sure no a versions are in any dependencies of stable releases (unless pinned specifically). For this PR, we could either push stable releases all of the added skills now, or include them in a subsequent PR removing alpha versions for all dependencies

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next one, this allows our community to test latest dev branch and report issues before we declare skills stable

@NeonDaniel NeonDaniel requested a review from a team March 6, 2023 21:16
@JarbasAl JarbasAl merged commit 1c134fd into dev Mar 7, 2023
@JarbasAl JarbasAl deleted the SKILLS_AddSkillRequirements branch March 7, 2023 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants